-
Notifications
You must be signed in to change notification settings - Fork 78
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New command: Type of dynamic selection #1675
base: master
Are you sure you want to change the base?
Conversation
301b8b0
to
dbb98d1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot wasn't able to review any files in this pull request.
Files not reviewed (16)
- package.json: Language not supported
- src-bindings/vscode/vscode.ml: Language not supported
- src-bindings/vscode/vscode.mli: Language not supported
- src/ast_editor.ml: Language not supported
- src/custom_requests.ml: Language not supported
- src/custom_requests.mli: Language not supported
- src/extension_commands.ml: Language not supported
- src/extension_consts.ml: Language not supported
- src/extension_instance.ml: Language not supported
- src/extension_instance.mli: Language not supported
- src/import.ml: Language not supported
- src/ocaml_lsp.ml: Language not supported
- src/ocaml_lsp.mli: Language not supported
- src/output.ml: Language not supported
- src/type_selection.ml: Language not supported
- src/vscode_ocaml_platform.ml: Language not supported
dbb98d1
to
baf6a20
Compare
@xvw, @PizieDust these changes are ready for review (but I cannot request it via the UI). |
New commands allow showing the type of the current selection, and grow / shrink that selection. Another commands increases the verbosity on demand.
eab4c59
to
6d7a3ed
Compare
module Handlers = struct | ||
let unpwrap = function | ||
| `Ok () -> () | ||
| `Error err_msg -> show_message `Error "%s" err_msg | ||
;; | ||
|
||
let w1 f x = | ||
try `Ok (f x) with | ||
| User_error e -> `Error e | ||
;; | ||
|
||
let ws f x y = | ||
match f x with | ||
| `Ok f' -> | ||
(try `Ok (f' y) with | ||
| User_error e -> `Error e) | ||
| `Error e -> `Error e | ||
;; | ||
|
||
let w2 f = ws (w1 f) | ||
let w3 f x = ws (w2 f x) | ||
let w4 f x y = ws (w3 f x y) | ||
let w5 f x y z = ws (w4 f x y z) | ||
let _w6 f x y z w = ws (w5 f x y z w) | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these standard names? I'm having trouble understanding what ws
and wX
mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, Just a little stressed by Handlers
.
This PR adds three new commands that can be used to get the type of a selection, and the ones of growing enclosings of this selection. At each step it is possible to ask for more verbose results.
The following videos show how the types of the growing selection are shown in a hover popup, with the selection growing, then shrinking, and lastly with an increased verbosity:
Screen.Recording.2024-12-04.at.11.28.03.mp4
The type history is also logged to an output channel which can be configured to show up automatically and clear itself to show only the last result.
I have been through a lot of back and forth to get to this result, because it is not possible (to the extent of my knowledge) to easily disable the standard hover provider from the client side due to how the official LSP client from Microsoft behaves. I tried multiple alternatives but I know think having a configuration option to mute the output of the standard hover from the server to be the most robust one.
Todo
cc @awilliambauer